feat: add persistent retry queue for failed telemetry events #6572
Conversation
- Implement TelemetryQueue class with FIFO queue structure - Add persistence using VSCode globalState - Integrate queue into TelemetryClient for automatic retry - Process pending events when new events are captured - Add comprehensive test coverage for queue functionality - Limit queue size to 1000 events and max 3 retries per event Fixes #4940
dosubot
bot
added
size:XXL
![roomote[bot]](https://avatars.githubusercontent.com/in/1546624?s=60&v=4){kind=small}
| await this.queue.enqueue(result.data) | ||
|
|
||
| // Process queue asynchronously if not already processing | ||
| if (!this.queue.isProcessingQueue()) { |
There was a problem hiding this comment.
Is this intentional? There's a potential race condition here between checking isProcessingQueue() and actually setting the processing state in processQueue(). Multiple simultaneous captures could trigger multiple queue processors.
Could we consider moving the processing state flag setting immediately after the check, or using a more atomic approach?
| }) | ||
|
|
||
| // Check if response indicates success (fetch doesn't throw on HTTP errors) | ||
| if (response === undefined || (response && response.ok !== false)) { |
There was a problem hiding this comment.
This condition seems a bit convoluted. Would this be clearer?
| if (response === undefined || (response && response.ok !== false)) { | |
| // Check if response indicates success | |
| if (response && response.ok) { |
The current logic treats undefined as success, which might be confusing for future maintainers.
|
|
||
| export class TelemetryQueue { | ||
| private static readonly QUEUE_KEY = "rooCode.telemetryQueue" | ||
| private static readonly MAX_QUEUE_SIZE = 1000 // Prevent unbounded growth |
There was a problem hiding this comment.
Could we consider making these limits configurable? Different deployment scenarios might benefit from different queue sizes and retry counts. Perhaps through extension settings or environment variables?
| * Processes the telemetry queue, sending events to the cloud service | ||
| */ | ||
| private async processQueue(): Promise<void> { | ||
| if (!this.authService.isAuthenticated()) { |
There was a problem hiding this comment.
I notice the queue doesn't automatically process when authentication state changes. If a user was offline and queued events, then comes online and authenticates, they'd need to trigger a new event before the queue processes. Should we consider adding a listener for auth state changes to process the queue?
hannesrudolph
added
the
Issue/PR - Triage
hannesrudolph
added
PR - Needs Preliminary Review
and removed
Issue/PR - Triage
daniel-lxs
moved this from PR [Needs Prelim Review]
to PR [Draft / In Progress]
in Roo Code Roadmap
hannesrudolph
added
PR - Draft / In Progress
and removed
PR - Needs Preliminary Review
labels
github-project-automation
bot
moved this from PR [Draft / In Progress]
to Done
in Roo Code Roadmap
This PR implements a persistent retry queue for telemetry events that fail to send to Roo Code Cloud, as requested in #4940.
Changes
Key Features
Testing
TelemetryQueue.test.tswith tests for all queue operationsTelemetryClient.queue.test.tsfor integration testingFixes #4940
Important
Introduces a persistent retry queue for telemetry events in
TelemetryClient, ensuring failed events are retried and not lost, with extensive test coverage.TelemetryQueueclass for persistent FIFO queue using VSCodeglobalState.TelemetryClientnow enqueues events instead of sending directly, with automatic retries up to 3 times.TelemetryClientmodified to useTelemetryQueuefor event handling.TelemetryQueue.test.tsfor queue operations.TelemetryClient.queue.test.tsfor integration testing.TelemetryClienttests for queue-based approach.This description was created by
for a629172. You can customize this summary. It will automatically update as commits are pushed.